Add WFC3 notebook 'calwf3_recal_tvb.ipynb' #99

dulude · 2023-10-05T15:34:58Z

Relevant Tickets

SPB-1619

This notebook checklist has been made available to us by the Notebooks For All team.
Its purpose is to serve as a guide for both the notebook author and the technical reviewer highlighting critical aspects to consider when striving to develop an accessible and effective notebook.

The First Cell

The title of the notebook in a first-level heading (eg. <h1> or # in markdown).
A brief description of the notebook.
A table of contents in an ordered list (1., 2., etc. in Markdown).
The author(s) and affiliation(s) (if relevant).
The date first published.
The date last edited (if relevant).
A link to the notebook's source(s) (if relevant).

The Rest of the Cells

There is only one H1 (# in Markdown) used in the notebook.
The notebook uses other heading tags in order (meaning it does not skip numbers).

Text

All link text is descriptive. It tells users where they will be taken if they open the link.
All acronyms are defined at least the first time they are used.
Field-specific/specialized terms are used when needed, but not excessively.

Code

Code sections are introduced and explained before they appear in the notebook. This can be fulfilled with a heading in a prior Markdown cell, a sentence preceding it, or a code comment in the code section.
Code has explanatory comments (if relevant). This is most important for long sections of code.
If the author has control over the syntax highlighting theme in the notebook, that theme has enough color contrast to be legible.
Code and code explanations focus on one task at a time. Unless comparison is the point of the notebook, only one method for completing the task is described at a time.

Images

All images (jpg, png, svgs) have an image description. This could be
- Alt text (an alt property)
- Empty alt text for decorative images/images meant to be skipped (an alt attribute with no value)
- Captions
- If no other options will work, the image is decribed in surrounding paragraphs.
Any text present in images exists in a text form outside of the image (this can be alt text, captions, or surrounding text.)

Visualizations

Fred's notes for wfc3_calwf3_recalibration_notebook:

Removed notebook from _config
Added notebook to _toc
Small grammar update in README
Added crds to requirements
Added pre-requirements to install hstcal
Updated notebook to PEP8 standards

…b from _toc.yml

…om exclude_patterns list in _config.yml

review-notebook-app · 2023-10-05T15:35:02Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

…_recal_tvb

Add calwf3 recal tvb

FDauphin · 2023-11-16T23:28:57Z

@haticekaratay @camipacifici @dulude I was able to pass the PEP8 style check, but failed the Notebook Execution and Validation.

In the raw .ipynb file, the Python version under metadata is 3.8.12, but CI is "Set up" in Python 3.11. Should I update that to Python 3.11?
Under "Execute notebooks", the CI returns a FileExistsError when I try to make a local directory named orig/. How can this subdirectory already exist?
The final error is under "Checkout only the file to the target branch". If the other two issues above are resolved, will this also resolve?

I appreciate any help towards fixing these small bugs

FDauphin · 2023-11-20T15:10:59Z

@haticekaratay @camipacifici @dulude I was able to pass the PEP8 style check, but failed the Notebook Execution and Validation.

In the raw .ipynb file, the Python version under metadata is 3.8.12, but CI is "Set up" in Python 3.11. Should I update that to Python 3.11?

Under "Execute notebooks", the CI returns a FileExistsError when I try to make a local directory named orig/. How can this subdirectory already exist?

The final error is under "Checkout only the file to the target branch". If the other two issues above are resolved, will this also resolve?

I appreciate any help towards fixing these small bugs

Hi @haticekaratay @dulude , I saw in #100 that the pre-requirements.sh bug was resolved so I came back here to check if the CI ran again. There were some minor conflicts that I resolved and the CI ran again, with all checks passing. However, it still produces an error on my second bullet point. Why would it pass if it found an error, and is it ok for review? I appreciate all the help!

haticekaratay · 2023-11-24T18:59:58Z

@haticekaratay @camipacifici @dulude I was able to pass the PEP8 style check, but failed the Notebook Execution and Validation.

In the raw .ipynb file, the Python version under metadata is 3.8.12, but CI is "Set up" in Python 3.11. Should I update that to Python 3.11?

Under "Execute notebooks", the CI returns a FileExistsError when I try to make a local directory named orig/. How can this subdirectory already exist?

The final error is under "Checkout only the file to the target branch". If the other two issues above are resolved, will this also resolve?

I appreciate any help towards fixing these small bugs

Hi @haticekaratay @dulude , I saw in #100 that the pre-requirements.sh bug was resolved so I came back here to check if the CI ran again. There were some minor conflicts that I resolved and the CI ran again, with all checks passing. However, it still produces an error on my second bullet point. Why would it pass if it found an error, and is it ok for review? I appreciate all the help!

Thanks, @FDauphin, for bringing to attention the issue of the CI showing all green checks despite errors. This issue has been reported to the CI team( @dulude, @mgough-970). Apart from that, I've provided some suggestions for enhancing the notebook. Thanks again for your work.

haticekaratay · 2023-11-24T20:30:26Z

notebooks/WFC3/calwf3_recalibration/calwf3_recal_tvb.ipynb

@@ -55,20 +55,18 @@
    "This notebook assumes you have created the virtual environment in [WFC3 Library's](https://github.com/spacetelescope/WFC3Library) installation instructions.\n",


In reviewing the part of the notebook where files are being renamed and moved, I'd like to suggest a small enhancement for clarity. The code snippet uses os.path.basename and os.path.join to handle file paths which is great practice for cross-platform compatibility and code readability.

for im in science_files: root = os.path.basename(im) new_path = os.path.join('.', root) os.rename(im, new_path)
data_directory......

Reply via ReviewNB

haticekaratay · 2023-11-24T20:30:26Z

notebooks/WFC3/calwf3_recalibration/calwf3_recal_tvb.ipynb

@@ -55,20 +55,18 @@
    "This notebook assumes you have created the virtual environment in [WFC3 Library's](https://github.com/spacetelescope/WFC3Library) installation instructions.\n",


Thanks for your work. I've reviewed the current approach for moving files, and I have a suggestion that might streamline the process, especially considering the potential for repeated runs of the notebook. To ensure that we efficiently handle files that may already exist in the destination directory, I propose the following code:

os.makedirs('orig/', exist_ok=True)
for file_pattern in ['ibohbf*_ima.fits', 'ibohbf*_flt.fits', 'ibohbf*_drz.fits']:
  for file in glob.glob(file_pattern):
    destination_path = os.path.join('orig', os.path.basename(file))
    if os.path.isfile(destination_path):
      os.remove(destination_path)
    shutil.move(file, destination_path)

Reply via ReviewNB

…t_notebooks into add_calwf3_recal_tvb

Add calwf3 recal tvb

Fixed indentation error for calwf3_recal notebook

FDauphin · 2023-11-27T21:25:26Z

@haticekaratay Thanks for the comments. I've integrated them into the notebook. However, now I have an error in the Greetings check, which I am unfamiliar with. What should I change so it passes? Thanks for the help.

Fixed indentation bug in calwf3_recal

haticekaratay

Thanks for your work @FDauphin. The Action greetings can be ignored for our case, and it's been reported to the CI team.

dulude added 3 commits October 5, 2023 11:26

uncommented notebooks/WFC3/calwf3_recalibration/calwf3_recal_tvb.ipyn…

c8a4853

…b from _toc.yml

removed notebooks/WFC3/calwf3_recalibration/calwf3_recal_tvb.ipynb fr…

e32e491

…om exclude_patterns list in _config.yml

calwf3_recal_tvb.ipynb: cleared notebook outputs

df8b0dd

dulude added the WFC3 Wide Field Camera 3 label Oct 5, 2023

dulude requested review from camipacifici and haticekaratay October 5, 2023 15:34

dulude assigned dulude and FDauphin Oct 5, 2023

dulude had a problem deploying to ci_env October 5, 2023 15:35 — with GitHub Actions Failure

FDauphin assigned mackjenn Oct 16, 2023

FDauphin and others added 8 commits November 16, 2023 17:47

Removed calwf3_recal notebook from _config

42e14b2

Uncommented calwf3_recal notebook from _toc

00d9b39

Update README for calwf3_recal

1ab622e

Update requirements for calwf3_recal

a2836c0

Added pre-requirements for calwf3_recal to install hstacl

0b44030

Update calwf3_recal notebook to PEP8 standards

97dabcf

Merge branch 'add_wfc3_calwf3_recalibration_notebook' into add_calwf3…

1393756

…_recal_tvb

Merge pull request #136 from FDauphin/add_calwf3_recal_tvb

150ae92

Add calwf3 recal tvb

FDauphin temporarily deployed to ci_env November 16, 2023 22:57 — with GitHub Actions Inactive

FDauphin had a problem deploying to ci_env November 16, 2023 22:57 — with GitHub Actions Failure

Merge branch 'main' into add_wfc3_calwf3_recalibration_notebook

99af657

FDauphin temporarily deployed to ci_env November 20, 2023 14:53 — with GitHub Actions Inactive

haticekaratay reviewed Nov 24, 2023

View reviewed changes

Updated README with dependency instructions

685694c

FDauphin and others added 3 commits November 27, 2023 15:34

Updated code for file naming/moving

c840360

Merge branch 'add_calwf3_recal_tvb' of https://github.com/FDauphin/hs…

4865a58

…t_notebooks into add_calwf3_recal_tvb

Merge pull request #145 from FDauphin/add_calwf3_recal_tvb

4155d24

Add calwf3 recal tvb

FDauphin had a problem deploying to ci_env November 27, 2023 21:03 — with GitHub Actions Failure

FDauphin temporarily deployed to ci_env November 27, 2023 21:03 — with GitHub Actions Inactive

FDauphin and others added 2 commits November 27, 2023 16:19

Fixed indentation error

49057b7

Merge pull request #146 from FDauphin/add_calwf3_recal_tvb

ac1f7bd

Fixed indentation error for calwf3_recal notebook

FDauphin temporarily deployed to ci_env November 27, 2023 21:24 — with GitHub Actions Inactive

FDauphin had a problem deploying to ci_env November 27, 2023 21:24 — with GitHub Actions Failure

FDauphin had a problem deploying to ci_env November 27, 2023 21:50 — with GitHub Actions Failure

Fix indentation in the loop

83fb973

haticekaratay temporarily deployed to ci_env November 27, 2023 22:34 — with GitHub Actions Inactive

FDauphin and others added 2 commits November 27, 2023 22:52

Fixed indentation bug

dbb166f

Merge pull request #147 from FDauphin/add_calwf3_recal_tvb

4e51a2d

Fixed indentation bug in calwf3_recal

FDauphin temporarily deployed to ci_env November 28, 2023 03:54 — with GitHub Actions Inactive

haticekaratay approved these changes Nov 28, 2023

View reviewed changes

haticekaratay merged commit 0a07a7c into main Nov 28, 2023
5 of 6 checks passed

mgough-970 deleted the add_wfc3_calwf3_recalibration_notebook branch December 6, 2023 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WFC3 notebook 'calwf3_recal_tvb.ipynb' #99

Add WFC3 notebook 'calwf3_recal_tvb.ipynb' #99

dulude commented Oct 5, 2023 •

edited by FDauphin

Loading

review-notebook-app bot commented Oct 5, 2023

FDauphin commented Nov 16, 2023

FDauphin commented Nov 20, 2023 •

edited

Loading

haticekaratay commented Nov 24, 2023

haticekaratay Nov 24, 2023 •

edited

Loading

haticekaratay Nov 24, 2023 •

edited

Loading

FDauphin commented Nov 27, 2023 •

edited

Loading

haticekaratay left a comment

		@@ -55,20 +55,18 @@
		"This notebook assumes you have created the virtual environment in [WFC3 Library's](https://github.com/spacetelescope/WFC3Library) installation instructions.\n",

Add WFC3 notebook 'calwf3_recal_tvb.ipynb' #99

Add WFC3 notebook 'calwf3_recal_tvb.ipynb' #99

Conversation

dulude commented Oct 5, 2023 • edited by FDauphin Loading

The First Cell

The Rest of the Cells

Text

Code

Images

Visualizations

review-notebook-app bot commented Oct 5, 2023

FDauphin commented Nov 16, 2023

FDauphin commented Nov 20, 2023 • edited Loading

haticekaratay commented Nov 24, 2023

haticekaratay Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

haticekaratay Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

FDauphin commented Nov 27, 2023 • edited Loading

haticekaratay left a comment

Choose a reason for hiding this comment

dulude commented Oct 5, 2023 •

edited by FDauphin

Loading

FDauphin commented Nov 20, 2023 •

edited

Loading

haticekaratay Nov 24, 2023 •

edited

Loading

haticekaratay Nov 24, 2023 •

edited

Loading

FDauphin commented Nov 27, 2023 •

edited

Loading